[#12048] V9: Cleanup and refactor #12090

samuelfangjw · 2023-02-15T15:44:53Z

Part of #12048

Periodic cleanup and refactor of v9-migration branch to keep things neat. Changes are as follows:

Remove builder pattern from entities.

We do not have a very strong reason to continue to use the builder pattern in our entities. Blindly applying design patterns is itself an anti-pattern.
Removal of this pattern allows us to more freely use whichever creational pattern best suits the needs of the particular entity in question.
It is easy to refactor the code to support this pattern in the future, if desired. However, removing something that is already in place is will be much more difficult.

Standardise use of wrapper type Integer for ids

These ids can be null at the point of creation (before they are inserted into the db). Wrapper types allow us to check for null instead of the default value 0.

Move sanitisation methods into setter.

With hibernate, any changes to managed entities are automatically pushed to the db when the persistent context is flushed. This means that we cannot reliably ensure that the sanitizeForSaving method is always called. For example, if an entity is modified without save being explicitly called, the entity will be updated in the db without having its fields sanitised.
We moved the sanitisation methods to the setters, so that it's easier to ensure that every field is properly sanitised at all times.

Use natural key for equals and hashcode methods

As recommended here: https://thorben-janssen.com/ultimate-guide-to-implementing-equals-and-hashcode-with-hibernate/

Standardised unit test case naming
Disable failing tests

All component/lint tests should now pass

src/main/java/teammates/storage/sqlentity/FeedbackSession.java

src/main/java/teammates/storage/sqlentity/Account.java

hhdqirui

LGTM

* [#12048] Set up github action workflows * [#12048] v9: Skeleton implementation (#12056) * [#12048] Add isMigrated flag to course (#12063) * [#12048] Add is migrated flag to datastore account (#12070) * Temporarily disable liquibase migrations * [#12048] Create Notification Entity for PostgreSQL migration (#12061) * [#12048] Create notification DB layer for v9 migration (#12075) * [#12048] Add UsageStatistics entity and db (#12076) * Migrate GetCourseAction.java * Migrate DeleteCourseAction.java and relevant logic functions * Migrate BinCourseAction.java and its related logic functions * Update checkSpecificAccessControl functions in BinCourseAction and DeleteCourseAction classes * Migrate RestoreCourseAction and its related logic functions * Migrate UpdateCourseAction with its related logic functions * [#12048] Add Account Entity (#12087) * [#12048] Create SQL logic for CreateNotificationAction and add relevant tests for v9 migration (#12077) * [#12048] Create Student, Instructor and User Entities for PostgreSQL Migration (#12071) * [#12048] V9: Cleanup and refactor (#12090) * Edit GetCourseAction and refactor out the old datastore code * [#12048] Remove redundant InstructorRole Enum (#12091) * Fix compilation error * Update check for database to fetch from * Add unit tests for CoursesDb * [#12048] Update GetUsageStatisticsAction to include SQL entities (#12084) * Add CoursesLogicTest class * Disable failing tests * Fix compilation error * Fix Checkstyle errors * Merge branch * Change flow for updating courses. * Update updateCourse JavaDoc comment. * Update CreateCourseAction and related methods * Update GetCourseAction. * Update UpdateCourseAction * Update BinCourseAction and RestoreCourseAction * Update DeleteCourseAction * Migrate GetCourseSectionNamesAction and related methods. * Add Unit tests for Logic layer of Course. * Fix Checkstyle errors * Add unit test for GetCourseAction's execute function * Add verify for CoursesDb unit tests and use assertNull and assertNotNull * Move fetching of course to logic layer. * Fix Checkstyle errors. * Move canCreateCourse logic to logic layer. * Change *CourseAction classes to use isCourseMigrated * Fix CoursesLogic's initLogicDependencies method call * Add unit tests for GetCourseAction. * Remove commented out method. * Add minimal unit tests for BinCourseAction, DeleteCourseAction and RestoreCourseAction. * Add minimal unit tests for GetCourseSectionAction and UpdateCourseAction. * Remove unused EntityType parameter. * Add minimal unit tests for CreateCourseAction. * Fix Checkstyle errors. * Ignore all old datastore test cases for *CourseAction classes. * Fix 'text' type to 'test'. * Change binCourseToRecycleBin to return the binned course. * Update moveCourseToRecycleBin test. * Update test name. --------- Co-authored-by: Samuel Fang <samuelfangjw@gmail.com> Co-authored-by: dao ngoc hieu <53283766+daongochieu2810@users.noreply.github.com> Co-authored-by: Samuel Fang <60355570+samuelfangjw@users.noreply.github.com> Co-authored-by: wuqirui <53338059+hhdqirui@users.noreply.github.com> Co-authored-by: Dominic Lim <46486515+domlimm@users.noreply.github.com>

samuelfangjw added 6 commits February 15, 2023 22:10

Remove builder pattern

a56956a

Standardize use of Integer for hibernate id

a0a7f3b

Move sanitization into setters

a2f9910

Use natural key for equals and hashcode

35ae086

Standardize unit test case naming

2d8d412

Disable failing testcases

c4b1dc8

samuelfangjw requested review from daongochieu2810 and hhdqirui February 15, 2023 15:44

daongochieu2810 reviewed Feb 15, 2023

View reviewed changes

src/main/java/teammates/storage/sqlentity/FeedbackSession.java Show resolved Hide resolved

src/main/java/teammates/storage/sqlentity/Account.java Show resolved Hide resolved

daongochieu2810 approved these changes Feb 15, 2023

View reviewed changes

hhdqirui approved these changes Feb 15, 2023

View reviewed changes

samuelfangjw merged commit 4b1e445 into TEAMMATES:v9-migration Feb 15, 2023

domlimm pushed a commit to domlimm/teammates that referenced this pull request Feb 22, 2023

[TEAMMATES#12048] V9: Cleanup and refactor (TEAMMATES#12090)

ca107a0

samuelfangjw added a commit that referenced this pull request Feb 26, 2023

[#12048] V9: Cleanup and refactor (#12090)

6b7c460

samuelfangjw added a commit that referenced this pull request Apr 8, 2023

[#12048] V9: Cleanup and refactor (#12090)

729db70

wkurniawan07 added s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging c.Task Other non-user-facing works, e.g. refactoring, adding tests labels Jan 21, 2024

wkurniawan07 added this to the V9.0.0-beta.0 milestone Jan 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#12048] V9: Cleanup and refactor #12090

[#12048] V9: Cleanup and refactor #12090

samuelfangjw commented Feb 15, 2023 •

edited

Loading

hhdqirui left a comment

[#12048] V9: Cleanup and refactor #12090

[#12048] V9: Cleanup and refactor #12090

Conversation

samuelfangjw commented Feb 15, 2023 • edited Loading

hhdqirui left a comment

Choose a reason for hiding this comment

samuelfangjw commented Feb 15, 2023 •

edited

Loading